Sample app builds 📱

Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.

• java_layout: rehan/push-click-behavior (1699951544)
• kotlin_compose: rehan/push-click-behavior (1699951564)

Codecov Report

Attention: 105 lines in your changes are missing coverage. Please review.

Comparison is base (c494bb0) 49.76% compared to head (2032b89) 53.59%.
Report is 23 commits behind head on main.

@@             Coverage Diff              @@
##               main     #247      +/-   ##
============================================
+ Coverage     49.76%   53.59%   +3.82%     
- Complexity      237      278      +41     
============================================
  Files           108      109       +1     
  Lines          2781     2812      +31     
  Branches        364      352      -12     
============================================
+ Hits           1384     1507     +123     
+ Misses         1282     1183      -99     
- Partials        115      122       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Build available to test
Version: rehan-push-click-behavior-SNAPSHOT
Repository: https://s01.oss.sonatype.org/content/repositories/snapshots/

@levibostian Yes the change prevents new activity from being tracked by CIO auto-screen tracking. The PR does exactly what is described in the task breakdown. Current approach is much better than solutions like hard-coding filters. And even if it adds a feature, it does without breaking existing functionality and requires minimal effort on our part. I don’t see any reason for not continuing with it. If you have any suggestions for improvement or see any issues with this approach, feel free to mention them. Otherwise, let’s continue the discussion in the parent ticket or on Slack.

I am open to a solution such as this one where an interface is used to determine if a screen should be tracked or not.

My hesitations come up from:

• The use of this new interface TrackableScreen in the sample apps. If we are to begin using TrackableScreen in the sample apps, that makes me think we are suggesting it as our recommended approach to doing automatic screenview tracking in the SDK. Especially with the use of getScreenName() where we are returning different values for certain fragments, TrackableScreen seems as though it's trying to fix an unrelated problem to push click behavior. I suggest we introduce a new feature such as this with a new ticket and new PR.
• The naming of TrackableScreen makes it seem as though you need to use it in order for a screen to be tracked. A customer might ask, "If you do not inherit TrackableScreen, will my screen still be tracked?". We can discuss naming if this feature is turned into a new ticket with new PR.

I have an idea to suggest. Would it work for a refactor to the PR where TrackableScreen is renamed to DoNotTrackScreen and the interface is internal or undocumented? If it's an interface that is currently only used internally (not used in sample apps), I would be OK with this. Then, we can expose this interface to the public and make it a documented feature in the future if we believe it's important to add.

Update: I find the rename to DoNotTrackScreen to be optional. As long as the interface is internal and removed from sample app use, I would be OK with the solution. I do agree, I find an interface to be better then a hard-coded list of Activities.

@levibostian I actually started implementing this with DoNotTrackScreen but ended up on TrackableScreen. Because I think the number of changes are almost same but TrackableScreen is more flexible. I'm okay with removing it from sample apps though if we have more votes on it.

For the interface, I don't think this can be confusing for customers because we are not yet going to add this in docs. If the customers find out the interface themselves, the doc on it already explains it is fully optional. We can discuss any improvements in docs or name on the file, if you have any ideas in my mind, please post it on TrackableScreen so it is easier to discuss in thread.

In general, I believe that code should not be added to a sample app unless it's a public facing feature that is fully documented.

Therefore, I would approve of the PR if TrackableScreen was internal (either in scope or a comment on it specifying that it's currently only used internally) and all references to it were removed from the sample apps.

As you have mentioned, I agree that if we do decide to make this a public feature add in the the future, naming and improvements could be discussed at that time. For internal use, this looks good to me.